Fix Windows installer machine scope PATH update#29169
Conversation
Update Podman Windows installer to update the PATH for all users if executed in machine scope mode. Added a new test that checks if Podman installation folder is in PATH. Fixes podman-container-tools#29160 Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
|
LGTM |
lstocchi
left a comment
lstocchi
left a comment
There was a problem hiding this comment.
Tested and it works fine but i have a doubt on IS_UPGRADE. Is it really needed?
| Condition="MSIINSTALLPERUSER=1 AND NOT IS_UPGRADE" > | ||
| <CreateFolder /> | ||
| <Environment Id="UpdatePath" Name="PATH" Action="set" Permanent="no" Part="last" Value="[INSTALLDIR]" /> | ||
| <Environment Id="UpdatePath" Name="PATH" Action="set" Permanent="no" Part="last" Value="[INSTALLDIR]" System="false"/> |
There was a problem hiding this comment.
Is true/false correct for System? In the doc wix3 doc i see it was a YesNoType https://docs.firegiant.com/wix3/xsd/wix/environment/ , then in wix4 it is marked as bool but in the description it keeps saying "no" is the default value so it makes me think it's still yes/no
There was a problem hiding this comment.
After testing it, it works fine so i guess the boolean is also accepted
| <Directory Id="EnvEntries"> | ||
| <Component Id="EnvEntriesComponent" Guid="b662ec43-0e0e-4018-8bf3-061904bb8f5b" Bitness="always64"> | ||
| <Component Id="EnvEntriesComponent" Guid="b662ec43-0e0e-4018-8bf3-061904bb8f5b" Bitness="always64" | ||
| Condition="MSIINSTALLPERUSER=1 AND NOT IS_UPGRADE" > |
There was a problem hiding this comment.
Is IS_UPGRADE actually needed? I did not find any mention of it in the doc. If it works as the name suggests, wouldn't it create a broken installation? I mean you want to install podman (an upgrade), it uninstall the old one, it clears PATH, then the new installation is performed but as there is NOT IS_UPGRADE the PATH update is skipped. I did a test with WIX_UPGRADE_DETECTED and it does not actually update the path. So i have the feeling it works bc IS_UPGRADE is not actually filled properly so it is always false
4 participants
Update Podman Windows installer to update the PATH for all users if executed in machine scope mode.
Added a new test that checks if Podman installation folder is in PATH.
Fixes #29160
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?